[Sping MVC(인증)] 김예진 미션 제출합니다. #160

dPwls0125 · 2025-07-01T11:25:13Z

안녕하세요 오찌!
Spring 인증 미션 제출합니다.

이번 미션을 통해 ArgumentResolver, Interceptor의 동작 방식과 사용 방법에 대해 제대로 다루어볼 수 있어 즐거웠습니다.

Ohzzi

수고 많으셨습니다 예진님. 전체적으로 리뷰는 비즈니스가 이뤄지는 영역의 구분과 위치에 대한 내용에 대한게 주를 이뤘습니다. 항상 정답은 없는 내용이지만, 이 부분을 생각해보는게 나중에 실무에 가서 정말 큰 도움이 되는 부분이라고 생각해요.

추가로 이번 미션을 하면서 어려웠거나 더 공부해보고 싶은 부분이 있었을까요? 말씀해주시면 해당 부분에 맞춰서 좀 더 깊은 리뷰를 들어가볼 수 있을 것 같아요!

Ohzzi · 2025-07-11T01:42:58Z

src/main/java/roomescape/member/MemberService.java

+    public LoginMember parseTokenAndGetMemberInfo(Cookie[] cookies) {
+
+        String token = "";
+        for (Cookie cookie : cookies) {
+            if (cookie.getName().equals("token"))
+                token = cookie.getValue();
+        }
+
+        if (token.isEmpty()) throw new IllegalArgumentException("로그인하지 않은 사용자입니다.");
+
+        return tokenParser.paseMemberInfo(token);
+    }


쿠키는 웹 기술의 영역이기 때문에 인터셉터에서 쿠키에서 원하는 값을 꺼내고 서비스에 전달해주는 형태로 구현해보는 것은 어떨까요? 지금은 너무 비즈니스 영역까지 들어온 느낌이에요. 토큰 값이 쿠키에 있든 리퀘스트바디로 넘어오든 MemberService 입장에서는 알 필요 없고 회원에 대한 책임만 가지면 되지 않을까요?

네 이 부분도 인증에 대한 책임과 회원 도메인에 대한 책임이 혼재되어있는 것 같습니다. MemberService에서 인증 관련 책임을 분리하여 인터셉터와 AuthService를 통해 인증 과정을 처리하도록 변경해보았습니다.
그리고 Cookie에서 token을 꺼내는 로직 자체를 인터셉터에 두어서 AuthService에는 token 값만 넘기도록 변경했습니다.
AuthService에서도 cookie에 대한 정보를 알 필요 없이 token을 파싱하는 책임만 가지면 된다는 생각입니다.

Ohzzi · 2025-07-11T01:44:03Z

src/main/java/roomescape/token/TokenParser.java

+    @Value("${roomescape.auth.jwt.secret}")
+    private String secretKey;
+
+    public LoginMember paseMemberInfo(String token) {


오타 있네요!

Ohzzi · 2025-07-11T01:44:41Z

src/main/java/roomescape/member/MemberService.java

+    private final TokenProvider tokenProvider;
+    private final TokenParser tokenParser;


개인적으로는 아예 토큰 관련된 의존성은 전부 멤버서비스 바깥에 있는 쪽을 더 선호(멤버서비스는 정말 회원에 대한 의존만)하기는 합니다. 이 의존성 관련해서는 정답은 없으니 다각도로 고민해보세요!

단순하게 생각해서 member의 정보를 바탕으로 인증을 진행하니 하나의 서비스에 넣자는 생각이었던 것 같습니다.
그런데 인증이라는 과정이 member 서비스에서만 발생하는 일은 아닌 만큼, 분리를 하는것이 맞다는 생각이 들었습니다.

멤버서비스는 순수한 회원 도메인 책임에만 집중시키고, 토큰 검증·관리 로직은 말씀하신 대로 별도의 인증 모듈(필터/인터셉터/AuthService)로 완전히 분리하는 방향으로 구조를 변경해보았습니다.

Ohzzi · 2025-07-11T01:45:18Z

src/main/java/roomescape/reservation/ReservationService.java

        return new ReservationResponse(reservation.getId(), reservationRequest.getName(), reservation.getTheme().getName(), reservation.getDate(), reservation.getTime().getValue());
    }

+    private ReservationRequest replaceNameIfEmpty(ReservationRequest request, LoginMember loginMember) {
+        if (request.getName() == null || request.getName().isBlank()) {


getName을 미리 변수로 꺼내놓으면 두 번 호출하지 않아도 되겠죠?

Ohzzi · 2025-07-11T01:52:47Z

src/main/java/roomescape/auth/LoginRequest.java

+public class LoginRequest {
+    private String email;
+    private String password;
+
+    public String getEmail() {
+        return email;
+    }
+
+    public String getPassword() {
+        return password;
+    }
+}


record를 사용해봐도 좋을 것 같네요

네 record를 사용해보았는데 getter, setter, toString(), equals(), hashCode() 등의 메서드를 반복적으로 작성해야 하는 번거로움을 줄여준다는 측면에서 훨씬 간결한 것 같습니다!

Ohzzi · 2025-07-11T01:53:44Z

src/main/java/roomescape/auth/LoginMember.java

+    private Long id;
+    private String name;
+    private String email;
+    private String role;


role은 enum인 것이 더 관리되기 쉽지 않을까요?

네 User, Admin과 같이 허용 된 역할만 명시적으로 관리하고 잘못된 값이 들어가는 것을 방지하기 위해서 ENUM을 사용하는 것이 좋을 것 같습니다!

Ohzzi · 2025-07-11T01:54:52Z

src/main/java/roomescape/member/MemberController.java

+    @PostMapping("/login")
+    public ResponseEntity<Void> login(@RequestBody LoginRequest loginRequest) {
+        TokenResponse token = memberService.getAccessToken(loginRequest.getEmail(), loginRequest.getPassword());
+        ResponseCookie cookie = ResponseCookie.from("token", token.getAccessToken())
+                .httpOnly(true)
+                .path("/")
+                .build();
+
+        return ResponseEntity.ok()
+                .header(HttpHeaders.SET_COOKIE, cookie.toString())
+                .build();
+    }
+
+    @GetMapping("/login/check")
+    public ResponseEntity<MemberResponse> loingCheck(HttpServletRequest request) {
+        Cookie[] cookies = request.getCookies();
+        LoginMember loginMember = memberService.parseTokenAndGetMemberInfo(cookies);
+        MemberResponse response = new MemberResponse(loginMember.getId(), loginMember.getName(), loginMember.getEmail());
+        return ResponseEntity.ok().body(response);
+    }
+


로그인 관련해서는 컨트롤러가 별도로 분리되는게 좋을 것 같아요! 회원 컨트롤러는 회원 매니징에 대해서만 갖는게 맞고 로그인은 인증인가의 영역이니까요!

Ohzzi · 2025-07-11T01:55:45Z

src/main/java/roomescape/token/TokenProvider.java

+                .claim("name", member.getName())
+                .claim("role", member.getRole())
+                .claim("email", member.getEmail())


토큰에 너무 많은 정보가 들어가는 것 아닐까요?

어차피 member정보를 매번 쿼리하느니 최대한 많은 정보를 token에 담아두는 것이 재사용하기 좋지 않을까? 하는 생각이었습니다.
지금 다시 생각해보니 보안 측면, 토큰 크기가 증가한다는 측면에서 token에 많은 정보를 담아두는 것이 좋지 않다는 생각이 드는 것 같습니다.

맞아요. 또한 토큰이 일회성이 아니고 어쨌든 만료까지 시간이 있는데 그 사이에 정보가 변경되었을 때의 정합성 문제도 있어요. 그래서 저는 극단적으로는 role 체크 까지도 DB에서 매번 꺼내서 해야 한다는 입장이기는 합니다!

Ohzzi · 2025-07-11T02:00:41Z

src/main/java/roomescape/member/MemberService.java

+
+    public LoginMember parseTokenAndGetMemberInfo(Cookie[] cookies) {
+
+        String token = "";


토큰이 없는 것은 null이 더 명시적이지 않을까 하는 생각이 듭니다. 빈 문자열은 정말 빈 문자열과 헷갈릴 수 있잖아요? (물론 이 경우 빈 문자열도 문제가 되어야 하는 것은 마찬가지겠지만)

네 토큰이 없는 경우에 대해서는 null로 인지하는 것이 더 의미적으로도 더 맞는 것 같다는 생각이 듭니다!
사실 빈 문자열을 생각 없이 선언했는데 null을 반환 할 것인지 빈 문자열을 반환 할 것인지의 문제도 잘 생각해봐야겠습니다.

dPwls0125

리뷰 반영 완료했습니다!

다만 signup 관련 활동들에 대해서 프론트에서 매번 "/login/check"을 호출하여 로그인 여부를 확인하는 구조인 것 같은데, 지금 상태로는 signup을 한다는 것 자체가 로그인을 안한 상태이다보니 계속 401 status가 발생하여 회원가입에 실패하고 있습니다.

signup html 에서 login check을 수행하는 의도를 아직 잘 모르겠어서 더 찾아보고 추가 수정하여 반영하겠습니다.

file-changed 부분에서 한번에 comment 올리려고 했는데 왜인지 아래 중복으로 comment가 써졌네요..🥲
아래 내용은 무시하셔도 괜찮을 것 같습니당

dPwls0125 · 2025-07-24T13:25:24Z

src/main/java/roomescape/member/MemberService.java

+    private final TokenProvider tokenProvider;
+    private final TokenParser tokenParser;


단순하게 생각해서 member의 정보를 바탕으로 인증을 진행하니 하나의 서비스에 넣자는 생각이었던 것 같습니다.
그런데 인증이라는 과정이 member 서비스에서만 발생하는 일은 아닌 만큼, 분리를 하는것이 맞다는 생각이 들었습니다.

멤버서비스는 순수한 회원 도메인 책임에만 집중시키고, 토큰 검증·관리 로직은 말씀하신 대로 별도의 인증 모듈(필터/인터셉터/AuthService)로 완전히 분리하는 방향으로 구조를 변경해보았습니다.

dPwls0125 · 2025-07-24T13:32:37Z

src/main/java/roomescape/member/MemberService.java

+    public LoginMember parseTokenAndGetMemberInfo(Cookie[] cookies) {
+
+        String token = "";
+        for (Cookie cookie : cookies) {
+            if (cookie.getName().equals("token"))
+                token = cookie.getValue();
+        }
+
+        if (token.isEmpty()) throw new IllegalArgumentException("로그인하지 않은 사용자입니다.");
+
+        return tokenParser.paseMemberInfo(token);
+    }


네 이 부분도 인증에 대한 책임과 회원 도메인에 대한 책임이 혼재되어있는 것 같습니다. MemberService에서 인증 관련 책임을 분리하여 인터셉터와 AuthService를 통해 인증 과정을 처리하도록 변경해보았습니다.
그리고 Cookie에서 token을 꺼내는 로직 자체를 인터셉터에 두어서 AuthService에는 token 값만 넘기도록 변경했습니다.
AuthService에서도 cookie에 대한 정보를 알 필요 없이 token을 파싱하는 책임만 가지면 된다는 생각입니다.

dPwls0125 · 2025-07-24T13:40:16Z

src/main/java/roomescape/token/TokenProvider.java

+                .claim("name", member.getName())
+                .claim("role", member.getRole())
+                .claim("email", member.getEmail())


어차피 member정보를 매번 쿼리하느니 최대한 많은 정보를 token에 담아두는 것이 재사용하기 좋지 않을까? 하는 생각이었습니다.
지금 다시 생각해보니 보안 측면, 토큰 크기가 증가한다는 측면에서 token에 많은 정보를 담아두는 것이 좋지 않다는 생각이 드는 것 같습니다.

dPwls0125 · 2025-07-25T05:39:23Z

src/main/java/roomescape/auth/LoginRequest.java

+public class LoginRequest {
+    private String email;
+    private String password;
+
+    public String getEmail() {
+        return email;
+    }
+
+    public String getPassword() {
+        return password;
+    }
+}


네 record를 사용해보았는데 getter, setter, toString(), equals(), hashCode() 등의 메서드를 반복적으로 작성해야 하는 번거로움을 줄여준다는 측면에서 훨씬 간결한 것 같습니다!

dPwls0125 · 2025-07-25T05:41:44Z

src/main/java/roomescape/member/MemberService.java

+
+    public LoginMember parseTokenAndGetMemberInfo(Cookie[] cookies) {
+
+        String token = "";


네 토큰이 없는 경우에 대해서는 null로 인지하는 것이 더 의미적으로도 더 맞는 것 같다는 생각이 듭니다!
사실 빈 문자열을 생각 없이 선언했는데 null을 반환 할 것인지 빈 문자열을 반환 할 것인지의 문제도 잘 생각해봐야겠습니다.

dPwls0125 · 2025-07-25T05:46:46Z

src/main/java/roomescape/auth/LoginMember.java

+    private Long id;
+    private String name;
+    private String email;
+    private String role;


네 User, Admin과 같이 허용 된 역할만 명시적으로 관리하고 잘못된 값이 들어가는 것을 방지하기 위해서 ENUM을 사용하는 것이 좋을 것 같습니다!

dPwls0125 added 3 commits July 1, 2025 17:43

1단계 : 로그인

8738242

2,3단계 : 로그인 리팩터링, 관리자기능

11e2ea3

refact : 테스트명 변경

7a6895e

Ohzzi suggested changes Jul 11, 2025

View reviewed changes

dPwls0125 commented Jul 25, 2025

View reviewed changes

dPwls0125 added 2 commits July 25, 2025 15:54

리뷰 반영

36ca88a

fix : web config 변경

86293d3

		private final TokenProvider tokenProvider;
		private final TokenParser tokenParser;


		public LoginMember parseTokenAndGetMemberInfo(Cookie[] cookies) {

		String token = "";

[Sping MVC(인증)] 김예진 미션 제출합니다. #160

Are you sure you want to change the base?

[Sping MVC(인증)] 김예진 미션 제출합니다. #160

Uh oh!

Conversation

dPwls0125 commented Jul 1, 2025

Uh oh!

Ohzzi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dPwls0125 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dPwls0125 left a comment •

edited

Loading